fix(tls12): reject oversized AEAD plaintext - #22
Merged
Conversation
🎉 All green!🛠️ No new code quality issues 🔗 Commit SHA: d1ad916 | Docs | Datadog PR Page | Give us feedback! |
thieman
marked this pull request as ready for review
June 25, 2026 14:45
aqian01
approved these changes
Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR changes
Closes #17.
This PR makes the TLS 1.2 AEAD decrypters reject records whose decrypted plaintext is larger than the TLS record-size limit.
The new check is applied in both TLS 1.2 AEAD decrypt paths:
If decryption succeeds but the plaintext length is greater than 16 KiB, the provider now returns
Error::PeerSentOversizedRecord.What is the TLS record-size limit?
TLS does not send an entire connection as one giant encrypted blob. It splits traffic into records. In TLS 1.2, the plaintext carried by one record is limited to 2^14 bytes, which is 16 KiB.
RFC 5246, the TLS 1.2 specification, says the record layer carries data in chunks of 2^14 bytes or less and that the TLSPlaintext length must not exceed 2^14: https://datatracker.ietf.org/doc/html/rfc5246#section-6.2.1
So after a TLS 1.2 record is decrypted, the provider should not hand rustls a plaintext record larger than that limit.
Why this matters
This crate plugs into rustls as a crypto provider. The provider is responsible for decrypting TLS records and returning plaintext messages back to rustls.
The built-in rustls providers enforce this limit in their TLS 1.2 AEAD decrypters. For comparison:
Before this PR,
rustls-cng-cryptodecrypted the record and returned the plaintext without this provider-side length check. That could make this provider accept an authenticated TLS 1.2 record that rustls' built-in providers would reject.Why this fix is the right thing to do
The fix follows the same pattern as rustls' built-in TLS 1.2 providers:
Error::PeerSentOversizedRecord;The error variant matters.
PeerSentOversizedRecordtells rustls this was a peer protocol violation, not a local CNG failure and not an authentication-tag failure.How to read this if you are not a crypto expert
This PR is about enforcing a size rule, not changing encryption itself.
Imagine TLS records as sealed envelopes. AEAD decryption verifies the seal and opens the envelope. This PR adds a check after opening: "Is the paper inside larger than TLS allows for one envelope?" If yes, rustls rejects the record even though the cryptographic tag was valid.
That matches the TLS 1.2 spec and rustls' own providers.
Validation
This PR adds a regression test that:
MAX_FRAGMENT_LEN + 1bytes;Error::PeerSentOversizedRecord.The test covers both AES-GCM and ChaCha20-Poly1305 TLS 1.2 AEAD paths.
A follow-up commit fixed a test-module import issue caught by CI. The intended behavior is unchanged.